Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User management UI #5793

Merged
merged 100 commits into from
Oct 11, 2024
Merged

User management UI #5793

merged 100 commits into from
Oct 11, 2024

Conversation

lovincyrus
Copy link
Contributor

@lovincyrus lovincyrus commented Sep 27, 2024

Closes #5805

This PR contains the initial version of user management. Users can now add new users, view pending invites and remove users all in the cloud. Scoped out usergroup management changes for follow-up.

  • user management
  • list org invites
  • prepend org or project to roles
  • rework add users
  • search
  • rework create a group
  • polish users typehead list
  • filter out users that are already in the group, shouldn't be in the suggestion to add list
  • random avatar color
  • fix horizontal line
  • fix User management UI #5793 (comment)

New components: Avatar, AvatarCircleList, Combobox, BasicTable

https://www.notion.so/rilldata/Dev-Notes-10dba33c8f578038a199f257639c577e

https://www.notion.so/rilldata/User-Management-UI-8d331b29d9b64d87bca066e06ef87f54

https://www.figma.com/design/Qt6EyotCBS3V6O31jVhMQ7/RILL-Latest?node-id=16418-864914&node-type=frame&t=4877tB4jsjaLhZeO-0

User management

CleanShot.2024-09-27.at.10.17.23.mp4

User groups management

CleanShot.2024-09-26.at.17.03.23.mp4

Users with pending invites

CleanShot.2024-09-27.at.11.59.40.mp4

@lovincyrus lovincyrus force-pushed the cyrus/user-management-ui branch 2 times, most recently from 9607a0b to 7c87407 Compare October 3, 2024 18:02
@lovincyrus lovincyrus changed the title User and user group management UI User management UI Oct 3, 2024
@lovincyrus
Copy link
Contributor Author

The org settings page will be released together with these changes. cc: @AdityaHegde

CleanShot 2024-10-03 at 17 15 39@2x

Copy link
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much needed functionality! Overall, this is great. Here's the start of a review.

UXQA:

  1. For consistency with the Project page's sub-pages, I'd wouldn't expect to see the horizontal line separating the header's top & bottom rows:
    image
  2. Every time I go to the user page, my Avatar is a different color
    image
    image
  3. What's the effect of setting an Org's description? If there is an effect, can we say so in subtext? If there isn't one, can we remove it?
    image
  4. When I create an email pill, clicking "Invite" to submit the form adds an extra line in the text box as a side effect. Jam.
  5. I noticed that Viewers are unable to see the Users page. They should also be able to see some read-only version of the table, so they can know who their teammates are (especially who are the Admins). This could go in a follow-on PR.
  6. I didn't realize that these Role cells had dropdown menus until I hovered over them. Is there supposed to be a border around each role?
    image

scripts/tsc-with-whitelist.sh Outdated Show resolved Hide resolved
web-common/src/components/button/Button.svelte Outdated Show resolved Hide resolved
@lovincyrus
Copy link
Contributor Author

  1. For consistency with the Project page's sub-pages, I'd wouldn't expect to see the horizontal line separating the header's top & bottom rows:
    image

I'm afraid I don't have context for this. This was previously implemented in #5458 and the changeset of this PR uncomments the changes.

Can you confirm if this is the correct behavior of the horizontal line? @jkhwu

CleanShot.2024-10-07.at.16.34.57.mp4

@lovincyrus
Copy link
Contributor Author

2. Every time I go to the user page, my Avatar is a different color
image
image

This is intentional.

@lovincyrus
Copy link
Contributor Author

3. What's the effect of setting an Org's description? If there is an effect, can we say so in subtext? If there isn't one, can we remove it?
image

This is also a changeset from #5458

https://www.figma.com/design/Qt6EyotCBS3V6O31jVhMQ7/RILL-Latest?node-id=11202-427942&node-type=frame&t=WjZyoufpATOEltFj-0

Should we remove the org description? @jkhwu

@lovincyrus
Copy link
Contributor Author

6. I didn't realize that these Role cells had dropdown menus until I hovered over them. Is there supposed to be a border around each role?
image

A chevron icon will be added for this!

@jkhwu
Copy link
Contributor

jkhwu commented Oct 8, 2024

Should we remove the org description?

I believe we included org description because it can be set in the CLI, but I'm not sure. If it's not a CLI function I'm pretty sure we can remove it, but if it is, I'll defer on whether it's ok to remove to @ericokuma

Can you confirm if this is the correct behavior of the horizontal line?

Confirming that we intended for there to only be one horizontal divider, underneath the tabs if there are tabs to display (for viewers they may only see that line under the top header. This is as shown in the mocks.

Every time I go to the user page, my Avatar is a different color.

My expectation would be that color assignment to each user is randomized, but that once assigned, that color stays with the user (until some future day when we allow users to sub in custom pics, or we just use their Google one)

@eokuma
Copy link

eokuma commented Oct 8, 2024

  1. Every time I go to the user page, my Avatar is a different color
image image

This is intentional.

Oh, I would also expect the avatar color to be the same each time

@eokuma
Copy link

eokuma commented Oct 8, 2024

Should we remove the org description?
I believe we included org description because it can be set in the CLI, but I'm not sure. If it's not a CLI function I'm pretty sure we can remove it, but if it is, I'll defer on whether it's ok to remove to @ericokuma

Can you confirm if this is the correct behavior of the horizontal line?
Confirming that we intended for there to only be one horizontal divider, underneath the tabs if there are tabs to display (for viewers they may only see that line under the top header. This is as shown in the mocks.

Every time I go to the user page, my Avatar is a different color.
My expectation would be that color assignment to each user is randomized, but that once assigned, that color stays with the user (until some future day when we allow users to sub in custom pics, or we just use their Google one)

I agree with all the above.

Also we can remove description (though we designed it because it is part of the CLI function). @ericpgreen2, what's your concern with having a description field?

@lovincyrus
Copy link
Contributor Author

  1. Every time I go to the user page, my Avatar is a different color
image image

This is intentional.

Oh, I would also expect the avatar color to be the same each time

To persist a color, we'd need to add a new field to User so the returning user will always have a persistent color. Is that what we want to take on? cc: @ericpgreen2

CleanShot 2024-10-08 at 08 58 01@2x

@ericpgreen2
Copy link
Contributor

To persist a color, we'd need to add a new field to User so the returning user will always have a persistent color. Is that what we want to take on? cc: @ericpgreen2

Simpler, we could deterministically select a color client-side based on the user name or email.

@ericpgreen2
Copy link
Contributor

Also we can remove description (though we designed it because it is part of the CLI function). @ericpgreen2, what's your concern with having a description field?

No concern with the description field, just thought we could hide it for now if it's not used anywhere yet.

@lovincyrus
Copy link
Contributor Author

To persist a color, we'd need to add a new field to User so the returning user will always have a persistent color. Is that what we want to take on? cc: @ericpgreen2

Simpler, we could deterministically select a color client-side based on the user name or email.

Sure, we can do that.

@lovincyrus
Copy link
Contributor Author

Also we can remove description (though we designed it because it is part of the CLI function). @ericpgreen2, what's your concern with having a description field?

No concern with the description field, just thought we could hide it for now if it's not used anywhere yet.

Removed

@eokuma
Copy link

eokuma commented Oct 8, 2024

Also we can remove description (though we designed it because it is part of the CLI function). @ericpgreen2, what's your concern with having a description field?

No concern with the description field, just thought we could hide it for now if it's not used anywhere yet.

Removed

Sorry Cyrus, Eric, Janet, and I synced, and we decided to keep the description field

Copy link
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left some code comments & here are some UXQA items:

  1. The Avatar is using a random color in favor of my Google avatar.
  2. The users table is not showing users directly added to a project (but if this is meant for a follow-on PR, that's ok)

scripts/tsc-with-whitelist.sh Outdated Show resolved Hide resolved
web-common/src/components/avatar/Avatar.svelte Outdated Show resolved Hide resolved
web-admin/src/routes/[organization]/-/users/+page.svelte Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming nit: I'm not sure I follow what makes this a "List"

web-common/src/components/table/BasicTable.svelte Outdated Show resolved Hide resolved
@lovincyrus lovincyrus merged commit 756095f into main Oct 11, 2024
4 checks passed
@lovincyrus lovincyrus deleted the cyrus/user-management-ui branch October 11, 2024 18:09
@lovincyrus lovincyrus mentioned this pull request Oct 11, 2024
6 tasks
k-anshul pushed a commit that referenced this pull request Oct 15, 2024
* clean up org tabs

* org users table

* user groups

* capitalize

* delete user group action

* fix user group deletion

* rerender

* class tweaks

* add basic table component

* apply basic table to project resources table

* set user group role

* add then set role for user group

* conditional item or checkbox item based on role

* revoke user group role

* bug fix

* rename user group dialog

* rework forms in user group dialogs

* user composite cell

* lint

* lint

* lint

* add user to org

* remove user from org

* fix disabled

* add avatar component

* set role for org member user

* lint

* disallow current user to change role or remove themselves

* show org invites

* fix interface

* add member to user group

* allow adding user to user group for verified users

* organize dialog footer actions

* add users to usergroup

* wip before edit user group dialog

* pending invite copy

* update user round for avatar placeholder when no alt

* allow add to group for current user

* edit user group dialog, remove user from group

* lint

* remove rename user group dialog

* polish user group member users list

* pass current user email down

* update copy

* org groups table role cell, set or update role

* clean up

* org users table role cell

* lint

* rework add users

* lint

* basic table tweaks, widthPercent addition

* lint

* search in users table, empty text

* search in groups table

* nit

* lint

* clean up, hide remove for all-users

* lint

* pre combobox

* use combobox, search and add users in create group dialog

* remove unused, clean up for all-users group

* lint

* ability to add users to usergroup in edit group dialog

* remove adding users to usergroup in create group dialog

* add avatar circle list, polish typeahead

* lint

* filter out users that are already in the group

* lint

* danger text for remove text button

* clean up

* fix input styles in combobox

* remove unused

* try

* lint

* bump typecheck nonsvelte files

* add scrollable table config, random color wip

* comment

* clean up

* clean up

* clean up

* scope out usergroup changes

* comment

* remove collaborator role option

* use random color, non-gray, non-primary-secondary

* clean up

* Revert "bump typecheck nonsvelte files"

This reverts commit e18a898.

* add chevron to role change

* Reapply "bump typecheck nonsvelte files"

This reverts commit 4a5418e.

* horizontal line feedback

* lint

* text danger

* deterministic bg color client side

* comment out desc from settings

* noop comment

* Revert "comment out desc from settings"

This reverts commit 3e4d6e2.

* feedback

* update src alt conditionals from avatar

* feedback

* fix rebase

* avoid prop drilling, feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cloud UI: User Management
4 participants